Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

try Suggestion from Stephen and use Write methods directly #1519

Closed
wants to merge 1 commit into from
Closed

try Suggestion from Stephen and use Write methods directly #1519

wants to merge 1 commit into from

Conversation

adamsitnik
Copy link
Contributor

@stephentoub +20k in JSON!

Before:

RequestsPerSecond:           1,149,856
Max CPU (%):                 99
WorkingSet (MB):             407
Avg. Latency (ms):           1
Startup (ms):                196
First Request (ms):          31.96
Latency (ms):                0.12
Total Requests:              17,362,051
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             4,001
Published Size (KB):         102,238
SDK:                         5.0.100-preview.5.20264.2
Runtime:                     5.0.0-preview.6.20262.14
ASP.NET Core:                5.0.0-preview.5.20255.6

After:

RequestsPerSecond:           1,171,304
Max CPU (%):                 100
WorkingSet (MB):             410
Avg. Latency (ms):           0.86
Startup (ms):                202
First Request (ms):          31.5
Latency (ms):                0.1
Total Requests:              17,686,603
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             4,001
Published Size (KB):         102,238
SDK:                         5.0.100-preview.5.20264.2
Runtime:                     5.0.0-preview.6.20262.14

With dotnet/runtime#36371

RequestsPerSecond:           1,203,964
Max CPU (%):                 100
WorkingSet (MB):             408
Avg. Latency (ms):           0.77
Startup (ms):                198
First Request (ms):          36.87
Latency (ms):                0.11
Total Requests:              18,179,315
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             4,001
Published Size (KB):         102,238
SDK:                         5.0.100-preview.5.20264.2
Runtime:                     5.0.0-preview.6.20262.14
ASP.NET Core:                5.0.0-preview.5.20255.6

@karelz

@stephentoub
Copy link
Contributor

Does this violate the TE spec?
"A JSON serializer must be used to convert the object to JSON."

@adamsitnik
Copy link
Contributor Author

"A JSON serializer must be used to convert the object to JSON."

It does. 1200k looks tempting, but I am afraid I should close this issue.

@adamsitnik adamsitnik closed this May 19, 2020
@benaadams
Copy link
Contributor

OTOH if it could be reframed using C# source generators that would probably be ok

Haven't looked to deeply at source generators, but an example of a deserializater using generators and System.Text.Json https://github.com/TomaszRewak/C-sharp-stack-only-json-parser

@benaadams
Copy link
Contributor

Related issue from @jkotas dotnet/runtime#1568; not sure if using source generators for System.Json is in the 5.0 timeframe?

@jkotas
Copy link

jkotas commented May 19, 2020 •
edited
Loading

Source generators for json serialization are not in .NET 5, unfortunately. I agree that source generators are the right way to get json serializers with best performance.

Note that the source generators should be able to do even better than the handwritten json writter code. They can pre-encode constant names to save even more cycles. The pre-encoding is not convenient to do by hand, but it is very simple to do in source generator. There are other tricks that we can consider for source generators, like having special methods on json writer that skip some of the validation that is important for hand-written code, but that is unnecessary for auto-generated code.

cc @steveharter

@adamsitnik adamsitnik deleted the jsonExperiment branch October 28, 2020 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@adamsitnik @stephentoub @benaadams @jkotas